Skip to content

Fix typo identified in #172 - #173

Merged
xrmx merged 7 commits into
elastic:mainfrom
dliappis:fix-mypy-again
Jun 26, 2025
Merged

Fix typo identified in #172#173
xrmx merged 7 commits into
elastic:mainfrom
dliappis:fix-mypy-again

Conversation

@dliappis

@dliappis dliappis commented Jun 25, 2025

Copy link
Copy Markdown
Contributor

This commit fixes a typo observed in
#172 (comment)

Also allows to simply the code without a need for a helper function.

@dliappis
dliappis requested a review from xrmx June 25, 2025 14:00
@dliappis dliappis self-assigned this Jun 25, 2025
@dliappis
dliappis marked this pull request as draft June 25, 2025 14:01
@dliappis
dliappis force-pushed the fix-mypy-again branch 2 times, most recently from 7a192ed to b4eb92b Compare June 25, 2025 14:12
This commit fixes a mistake observed in
elastic#172 (comment)

and allows to simply the code further (no need for a helper function)
Comment thread .pre-commit-config.yaml
- id: check-bash-syntax
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.910
rev: v1.16.1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that we are currently installing the latest version of mypy when executing the various Python 3.x tests using nox (e.g. see https://github.com/elastic/ecs-logging-python/blob/main/noxfile.py#L54) hence there was a discrepancy here when running precommit tests, that was surfacing non-existent errors.

@dliappis
dliappis marked this pull request as ready for review June 25, 2025 14:54
@dliappis

Copy link
Copy Markdown
Contributor Author

@xrmx please take a look at this PR when you can; I think it fixes the typo/issue you observed in #172 and also various mypy issues due to different versions used in different tests.

Comment thread ecs_logging/_stdlib.py Outdated
xrmx
xrmx previously approved these changes Jun 26, 2025
Comment thread ecs_logging/_stdlib.py Outdated

@xrmx xrmx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick turnaround!

@xrmx
xrmx enabled auto-merge (squash) June 26, 2025 07:19
@xrmx
xrmx merged commit 5941904 into elastic:main Jun 26, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants